crates/cardano/src/ashard/rewards.rs (1)

56-77: Stale EWRAP reference in comment.

The comment at lines 59–61 still says "registered at RUPD startStep time but deregistered before EWRAP". After this refactor the per-account registration check happens during AShard, not Ewrap. Suggest updating to "deregistered before AShard" (or "before the boundary's per-account leg") to keep the comment in lockstep with the renamed pipeline.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/ashard/rewards.rs` around lines 56 - 77, Update the stale
comment referencing "EWRAP" to reflect the new pipeline stage: change the text
around the account deregistration explanation to say "deregistered before
AShard" (or "before the boundary's per-account leg") so it matches where the
per-account registration check now runs; the relevant area to edit is the
comment above the account.is_registered() branch in the function that calls
reward.total_value() and ctx.rewards.return_reward_to_treasury(total). Ensure
the updated comment keeps the existing clarification about pre-filtered accounts
and returned_rewards behavior.

crates/cardano/src/ashard/loading.rs (1)

19-57: Optional: drop the Option<Range<EntityKey>> wrapper.

The doc-comment already states load_ashard is the only caller and always passes Some(range). Making the parameter Range<EntityKey> directly removes a never-taken branch and tightens the contract. If a future "load all" caller appears, reintroducing the option is trivial.

♻️ Suggested refactor

-    fn load_pending_rewards_range<D: Domain>(
-        &mut self,
-        state: &D::State,
-        range: Option<Range<EntityKey>>,
-    ) -> Result<(), ChainError> {
-        let pending_iter = state
-            .iter_entities_typed::<PendingRewardState>(PendingRewardState::NS, range)?;
+    fn load_pending_rewards_range<D: Domain>(
+        &mut self,
+        state: &D::State,
+        range: Range<EntityKey>,
+    ) -> Result<(), ChainError> {
+        let pending_iter = state
+            .iter_entities_typed::<PendingRewardState>(PendingRewardState::NS, Some(range))?;

And drop the Some(range.clone()) at the call site (line 81) to pass range.clone() directly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/ashard/loading.rs` around lines 19 - 57, The function
load_pending_rewards_range currently accepts Option<Range<EntityKey>> but the
only caller (load_ashard) always passes Some(range), so tighten the signature to
take Range<EntityKey> directly: change the parameter type in
load_pending_rewards_range and update callers (e.g., in load_ashard remove
Some(range.clone()) and pass range.clone() directly); ensure the body drops any
pattern-matching for None (no-op) and uses the range value when calling
state.iter_entities_typed::<PendingRewardState>(PendingRewardState::NS, range).

tests/epoch_pots/main.rs (1)

500-513: Optional: collapse the two if let Some(boundary) checks.

The shard-index==0 reset and the per-shard extend can share a single boundary binding, which trims a branch and makes the relationship between "capture ending epoch" and "extend rewards" obvious.

♻️ Suggested refactor

                 CardanoWorkUnit::AShard(shard) => {
-                    if shard.shard_index() == 0 {
-                        // First shard of this boundary — reset accumulator
-                        // and capture the ending epoch.
-                        accumulated_applied.clear();
-                        if let Some(boundary) = shard.boundary() {
-                            accumulated_ending_epoch =
-                                Some(boundary.ending_state().number);
-                        }
-                    }
-                    if let Some(boundary) = shard.boundary() {
+                    if let Some(boundary) = shard.boundary() {
+                        if shard.shard_index() == 0 {
+                            // First shard of this boundary — reset
+                            // accumulator and capture the ending epoch.
+                            accumulated_applied.clear();
+                            accumulated_ending_epoch =
+                                Some(boundary.ending_state().number);
+                        }
                         accumulated_applied.extend(boundary.applied_rewards.iter().cloned());
                     }
                 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/epoch_pots/main.rs` around lines 500 - 513, Collapse the two separate
`if let Some(boundary)` checks into a single binding for `boundary` when
matching `CardanoWorkUnit::AShard(shard)`: inside one `if let Some(boundary) =
shard.boundary()` block first check `if shard.shard_index() == 0` to clear
`accumulated_applied` and set `accumulated_ending_epoch =
Some(boundary.ending_state().number)`, then unconditionally call
`accumulated_applied.extend(boundary.applied_rewards.iter().cloned())`; this
removes the duplicated boundary lookup and makes the reset-and-extend logic
contiguous.

crates/cardano/work_units.md (1)

5-11: Add a language to the fenced block (MD040).

markdownlint-cli2 flagged this fence as missing a language. Use text (or any non-rendered identifier) so the diagram passes lint.

📝 Suggested fix

-```
+```text
 Estart  →  Roll …  →  Rupd  →  Roll …  →  AShard ×N  →  Ewrap
 (open)     (blocks)   (RUPD)   (blocks)    (per-account)         (global + close)
                                                                           │
                                                                           ▼
                                                                   next epoch's Estart

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @crates/cardano/work_units.md around lines 5 - 11, The fenced code block
containing the epoch diagram starting with "Estart → Roll … → Rupd → Roll
… → AShard ×N → Ewrap" is missing a language tag (MD040); fix it by adding a
language identifier (use "text") immediately after the opening triple backticks
so the block becomes ```text and the diagram passes markdownlint-cli2
validation.

</details>

</blockquote></details>
<details>
<summary>crates/cardano/src/lib.rs (2)</summary><blockquote>

`284-341`: **Consolidate duplicated `effective_account_shards` lookup logic.**

The inline computation at lines 338–341 is identical to the new helper `read_effective_account_shards` (lines 253–258). On top of that, `load_epoch::<D>(state)` is invoked twice within `initialize` (line 292 for the warnings, line 338 to derive `effective_account_shards`). A small refactor avoids the double read and keeps the “persisted total else config” rule in one place.



<details>
<summary>♻️ Proposed refactor</summary>

Lift the helper to a free fn and reuse it (and reuse the loaded `EpochState` for the warnings):

```diff
-    /// Compute the effective `account_shards` value: stored
-    /// `ashard_progress.total` if a boundary is in flight, otherwise the
-    /// configured value.
-    fn read_effective_account_shards<D: Domain>(&self, state: &D::State) -> u32 {
-        load_epoch::<D>(state)
-            .ok()
-            .and_then(|e| e.ashard_progress.as_ref().map(|p| p.total))
-            .unwrap_or_else(|| self.config.account_shards())
-    }
+    /// Compute the effective `account_shards` value: stored
+    /// `ashard_progress.total` if a boundary is in flight, otherwise the
+    /// configured value.
+    fn read_effective_account_shards<D: Domain>(&self, state: &D::State) -> u32 {
+        effective_account_shards_from_state::<D>(state, &self.config)
+    }
+}
+
+fn effective_account_shards_from_state<D: Domain>(state: &D::State, config: &CardanoConfig) -> u32 {
+    load_epoch::<D>(state)
+        .ok()
+        .and_then(|e| e.ashard_progress.as_ref().map(|p| p.total))
+        .unwrap_or_else(|| config.account_shards())
 }
```

And in `initialize`, compute once and reuse the loaded epoch for both the warnings and the cached value.
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/lib.rs` around lines 284 - 341, Extract and reuse the
epoch load and the effective-account-shards logic: in initialize(), call
load_epoch::<D>(state) once and store the resulting EpochState (if Ok) for the
warnings; replace the inline computation of effective_account_shards with a call
to the existing helper read_effective_account_shards (or lift that helper to a
free fn if necessary) so the "persisted total else config" rule is implemented
in one place; update references to use the single loaded epoch value for the
tracing::warn! blocks and derive effective_account_shards from
read_effective_account_shards(state, &config) (or equivalent) to remove the
duplicate load_epoch::<D>(state) call.
```

</details>

---

`404-408`: **Per-`pop_work` state read may be unnecessary outside boundaries.**

`effective_account_shards` is refreshed on every `pop_work` call (each block cycle) via a `read_entity_typed` lookup, but it only matters at epoch boundaries. The cost is small (single point read), but you could skip the read when the cached value is non-stale by only refreshing when `ashard_progress` could have changed (i.e., before an actual boundary handoff). Worth considering if `pop_work` shows up in profiling.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/lib.rs` around lines 404 - 408, The code refreshes
self.effective_account_shards on every pop_work by calling
read_effective_account_shards::<D>(domain.state()), but this value only matters
when the epoch boundary handoff (EpochState.ashard_progress.total) can change;
avoid the repeated read by first checking whether ashard_progress has changed
and only calling read_effective_account_shards when it has. Add a small cached
marker on the struct (e.g., last_ashard_total or last_ashard_progress) and in
pop_work compare domain.state().ashard_progress.total (or the appropriate
accessor) to that cached value; if different, call read_effective_account_shards
and update both effective_account_shards and the cached marker, otherwise skip
the read and keep the existing effective_account_shards.
```

</details>

</blockquote></details>
<details>
<summary>crates/cardano/src/ewrap/commit.rs (1)</summary><blockquote>

`91-100`: **Full-table account scan to apply a handful of MIR rewards.**

`stream_and_apply_namespace::<D, AccountState>(state, &writer, None)` walks every account in state to surface the small set with MIR-queued deltas. The inline comment acknowledges this is "effectively a targeted write via the streaming path", but on mainnet-sized account tables this is a significant cost paid every boundary, regardless of how few MIRs are pending (often zero).

Consider a direct path: iterate `self.applied_mir_credentials` (or the credentials of the queued `AssignRewards` deltas), point-read each `AccountState`, apply, and write. The streaming helper can stay for the full-set namespaces (Pool/DRep/Proposal/EpochState), but accounts during Ewrap are inherently sparse.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/ewrap/commit.rs` around lines 91 - 100, The current call
to stream_and_apply_namespace::<D, AccountState>(state, &writer, None) performs
a full-table scan of AccountState each boundary; instead implement a targeted
path that iterates over self.applied_mir_credentials (or the credentials from
queued AssignRewards deltas), point-reads each AccountState, applies the MIR
delta, and writes the result via the same writer so only affected accounts are
touched; keep stream_and_apply_namespace for full-set namespaces like
Pool/DRep/Proposal/EpochState, and ensure the new code reuses the existing
apply/serialize logic used by stream_and_apply_namespace to maintain invariants
and error handling.
```

</details>

</blockquote></details>
<details>
<summary>crates/cardano/src/ashard/work_unit.rs (1)</summary><blockquote>

`62-129`: **Avoid recomputing `total_shards`/`range` from state in `commit_state`.**

`load` computes `total_shards` and `range` from persisted state, then calls `BoundaryWork::load_ashard(..., shard_index, total_shards, range)` — so the boundary already owns these values. `commit_state` repeats the same state read and recomputation, with two minor concerns:

1. **Code duplication**: identical 8-line block in both methods (and a third copy in `crates/cardano/src/lib.rs`).
2. **Subtle drift risk**: if `commit_state` ever observed a different `ashard_progress.total` than `load` did (e.g., a shard's apply landing between the two phases of the same work unit), the recomputed `range` would diverge from what was loaded. Today this can't happen, but cross-phase reuse of the captured values is more obviously correct.

Consider exposing the captured shard info from `BoundaryWork` (or storing the computed `range` in `AShardWorkUnit` after `load`) and have `commit_state` reuse it instead of re-deriving.



<details>
<summary>♻️ Sketch</summary>

```diff
 pub struct AShardWorkUnit {
     slot: BlockSlot,
     config: CardanoConfig,
     genesis: Arc<Genesis>,
     shard_index: u32,
-    boundary: Option<BoundaryWork>,
+    boundary: Option<BoundaryWork>,
+    /// Captured during `load` so `commit_state` reuses the exact same range.
+    range: Option<std::ops::Range<dolos_core::EntityKey>>,
 }
```

Then `load` stores `self.range = Some(range.clone())` and `commit_state` consumes it.
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/ashard/work_unit.rs` around lines 62 - 129, The
load/commit_state duplication and drift risk can be fixed by capturing the
computed total_shards and range during load and reusing them in commit_state
instead of re-reading state: after computing total_shards and range in load
(before calling BoundaryWork::load_ashard), store the values on the work unit
(e.g., add fields self.total_shards and/or self.range and assign them there),
and then change commit_state to take the boundary via self.boundary.as_mut() and
use the stored range/total_shards when calling boundary.commit_ashard::<D>(...)
rather than recomputing via load_epoch; update BoundaryWork usage only to
consume the already-stored range if needed.
```

</details>

</blockquote></details>
<details>
<summary>crates/cardano/src/work.rs (2)</summary><blockquote>

`276-279`: **Test `TEST_TOTAL_SHARDS = 1` doesn't cover multi-shard sequencing in this state machine.**

These tests collapse `AShard` into the `EWrap` tag and set `TEST_TOTAL_SHARDS = 1`, so the multi-shard loop in `pop_work` (lines 226–245) — including the `next_index >= total_shards` transition — is exercised only with `total_shards = 1`. Worth adding at least one test with `TEST_TOTAL_SHARDS > 1` that asserts the work buffer emits exactly N `AShard` units before transitioning to `EwrapBoundary` (and that `last_point_seen` / cursor invariants hold across that transition). This catches off-by-one and ordering regressions in the boundary state machine that the current tests would silently miss.



Want me to draft this test?


Also applies to: 325-330

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/work.rs` around lines 276 - 279, Add a new unit test that
sets TEST_TOTAL_SHARDS > 1 (instead of the current const TEST_TOTAL_SHARDS = 1)
to exercise the multi-shard sequencing in the pop_work logic: drive the
WorkBuffer/state machine (invoking pop_work) and assert that it emits exactly N
consecutive AShard units (N == TEST_TOTAL_SHARDS) before producing an
EwrapBoundary (or EWrap/Ewrap as used in this code), and verify that
last_point_seen and any cursor invariants are preserved across the transition;
target the test to exercise the pop_work path that contains the next_index >=
total_shards branch and use the existing helpers/setup used by the other tests
so it integrates with the same state machine and coverage.
```

</details>

---

`120-143`: **`account_shards == 0` "skip-shards" fallback contradicts `validate_total_shards`.**

`crates/cardano/src/ashard/shard.rs::validate_total_shards` rejects `0` outright ("account_shards must be >= 1"), yet both `on_ewrap_boundary` and the `PreEwrapBoundary` arm of `pop_work` treat `0` as "no shards — go straight to Ewrap". If a config bug or missing validation ever hands `0` here, you'll silently skip per-account boundary processing instead of failing fast. The defensive comment ("Shouldn't happen with a valid config") tacitly acknowledges this.

Either drop the branches and let an `unreachable!()` / `assert!(account_shards >= 1)` fail loudly, or change `validate_total_shards` to accept `0` as "no shards" if that's the intended semantics. Right now the two pieces disagree.




Also applies to: 210-225

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/work.rs` around lines 120 - 143, The code currently treats
account_shards == 0 as a valid "no shards" path in on_ewrap_boundary (and
similarly in the PreEwrapBoundary arm of pop_work), which contradicts
validate_total_shards that requires account_shards >= 1; change the behavior to
fail-fast: remove the special-case branch that returns EwrapBoundary when
account_shards == 0 and instead assert or panic if account_shards < 1 (e.g., add
assert!(account_shards >= 1) at the start of on_ewrap_boundary), keeping the
normal Restart -> AShardingBoundary path for valid shard counts; alternatively,
if the intended semantics are that 0 means "no shards", update
validate_total_shards to accept 0 and document that invariant—pick one
consistent approach and apply the same fix in the PreEwrapBoundary/pop_work code
paths so both agree with validate_total_shards.
```

</details>

</blockquote></details>
<details>
<summary>crates/cardano/src/ashard/shard.rs (1)</summary><blockquote>

`37-60`: **`debug_assert!` makes invalid `total_shards` a runtime div-by-zero in release.**

The contract is documented and there's a `validate_total_shards` helper, but the only enforcement inside `shard_key_range` is `debug_assert!`. In release builds, `total_shards == 0` reaches `let step = PREFIX_SPACE / total_shards;` and panics with a less informative message; non-divisor values silently produce truncated `step` arithmetic.

Since this is a public function and the cost of a real assertion (or returning `Result`) is negligible compared to the I/O the caller does, prefer `assert!` (or propagate the error from `validate_total_shards`).



<details>
<summary>🛡️ Proposed change</summary>

```diff
 pub fn shard_key_range(shard_index: u32, total_shards: u32) -> Range<EntityKey> {
-    debug_assert!(validate_total_shards(total_shards).is_ok());
-    debug_assert!(shard_index < total_shards);
+    assert!(
+        validate_total_shards(total_shards).is_ok(),
+        "invalid total_shards: {total_shards}"
+    );
+    assert!(shard_index < total_shards, "shard_index {shard_index} >= total_shards {total_shards}");
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/ashard/shard.rs` around lines 37 - 60, The function
shard_key_range uses debug_assert! to check validate_total_shards(total_shards)
and shard_index < total_shards, which leaves a division-by-zero and silent
truncation in release; change the debug_assert! checks to real runtime checks
(e.g., assert!(validate_total_shards(total_shards).is_ok()) and
assert!(shard_index < total_shards)) or convert the function to return Result
and propagate validate_total_shards() error from shard_key_range so PREFIX_SPACE
/ total_shards cannot divide by zero and invalid total_shards are handled
deterministically; update callers if you choose the Result approach and keep
references to shard_key_range, validate_total_shards, and PREFIX_SPACE when
making the change.
```

</details>

</blockquote></details>
<details>
<summary>crates/cardano/src/ewrap/loading.rs (1)</summary><blockquote>

`280-285`: **`shard_applied_*` fields are dead state on Ewrap-constructed `BoundaryWork`.**

`new_empty` zero-initializes `shard_applied_effective`, `shard_applied_unspendable_to_treasury`, and `shard_applied_unspendable_to_reserves`, but Ewrap never reads/writes them — the per-shard accumulators land in `EpochState.end` via `EpochEndAccumulate` and are read from `ending_state.end` by `wrapup.flush`. The fields exist only because `BoundaryWork` is shared with the AShard path.

Not a bug, but the dual-use API obscures intent. If a future change adds a third caller, it's easy to misuse these fields. Consider either splitting `BoundaryWork` into `AShardWork`/`EwrapWork` or documenting on the struct which fields each phase owns.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/ewrap/loading.rs` around lines 280 - 285, BoundaryWork's
shard_applied_effective / shard_applied_unspendable_to_treasury /
shard_applied_unspendable_to_reserves are dead for the Ewrap path (zeroed in
new_empty and never read by Ewrap; real per-shard accumulators live in
EpochState.end via EpochEndAccumulate and are consumed by wrapup.flush), so fix
by either splitting the dual-purpose struct into two clear types (e.g.,
AShardWork and EwrapWork) and move the AShard-only shard_applied_* fields into
AShardWork, or add explicit documentation/comments on BoundaryWork and those
three fields noting they are owned/used only by the AShard path; update
new_empty, any constructors, and usages of BoundaryWork (including references in
EpochEndAccumulate and wrapup.flush) to use the new types or to reflect the
documented ownership to prevent future misuse.
```

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @crates/cardano/src/model/epochs.rs:

• Around line 737-740: The apply path currently does
  entity.end.as_mut().expect("ESTART seeded EpochState.end before shards run")
  which panics if EpochState.end is None; instead ensure end is initialized to
  EndStats::default() when missing (so subsequent EpochWrapUp can overwrite it).
  Update the apply code in crates/cardano/src/model/epochs.rs to replace the
  expect with a safe initialization (e.g., use get_or_insert_with / get_or_insert
  or similar) so entity.end becomes Some(EndStats::default()) if it was None
  before mutating it.
• Around line 684-771: EpochEndAccumulate::undo currently assumes apply always
  mutated state, causing underflow/corruption when apply returned early; fix by
  recording in apply whether it performed mutations and the prior values needed to
  revert (capture previous end.effective_rewards, end.unspendable_to_treasury,
  end.unspendable_to_reserves and previous entity.ashard_progress/AShardProgress)
  into fields on EpochEndAccumulate (or a was_applied flag plus prev_* fields),
  then make undo a no-op if apply was skipped or restore those captured prev_*
  values and previous ashard_progress instead of unconditionally subtracting and
  setting committed/total; update apply (function EpochEndAccumulate::apply) to
  populate these fields only when it actually mutates state and add a proptest
  exercising apply→undo when ashard_progress is already advanced past
  completed_shard_index to lock the behavior.

In @crates/core/src/config.rs:

• Around line 988-1002: Add runtime validation to reject invalid account_shards
  values by calling the existing validate_total_shards() from
  crates/cardano/src/ashard/shard.rs during config initialization: when
  constructing/using CardanoConfig (e.g., inside CardanoLogic::initialize() before
  any use of CardanoConfig::account_shards() or DEFAULT_ACCOUNT_SHARDS), call
  validate_total_shards(self.account_shards.unwrap_or(Self::DEFAULT_ACCOUNT_SHARDS))
  and return/propagate an error if it fails; ensure the initialization path stops
  the startup with a clear error rather than allowing invalid values (0 or
  non-divisors of 256) to be used.

In @skills/debug-epoch-mismatch/SKILL.md:

• Line 138: The documentation still treats reward application as a single EWRAP
  work unit but the code split moves per-account reward application into AShard
  while Ewrap now handles MIR/global closure; update the tables that list or
  reference EWRAP (notably the "Classify the Mismatch", "Dolos Work Units →
  Haskell Concepts", and the instrumentation hypothesis table) to distinguish
  AShard vs Ewrap: point applyRUpd + reward filtering, unspendable routing and
  registration filtering to AShard and point MIR and boundary close to Ewrap, and
  adjust any bisection/debugging guidance so it routes to AShard for per-account
  reward issues and to Ewrap for MIR/global closure issues. Ensure references
  include the file/class names AShard, Ewrap and the function/concept names
  applyRUpd, MIR, reward filtering, boundary close to make locating code
  straightforward.

---

Outside diff comments:
In @crates/cardano/src/ashard/rewards.rs:

• Around line 114-188: The per-shard logs in flush don't indicate which shard
  they belong to; update the flush logging to include shard context (shard_index
  and total_shards). Add shard_index: usize and total_shards: usize as fields on
  the AShard struct (or otherwise make them available on self or ctx), ensure any
  AShard construction/population passes these values, and then include
  %shard_index and %total_shards in the tracing::debug! and tracing::error!
  invocations (and the debug! at the end) around ctx.rewards/ drain_unspendable/
  applied_reward_credentials so all per-shard telemetry shows shard scope.

In @crates/cardano/src/model/epochs.rs:

• Around line 980-981: The rustdoc warning comes from using a /// doc comment
  immediately above the prop_compose! macro (related to EpochStatsUpdate::apply
  and rolling.live_mut which asserts rolling.next is None), so replace that
  leading /// comment with a non-doc comment (//) or move the explanation into the
  generator body/closure so rustdoc won't try to document the macro invocation;
  specifically update the comment above the prop_compose! invocation that
  references rolling.live_mut and rolling.next to use // (or relocate it inside
  the prop_compose! closure) to silence the unused_doc_comments lint.

---

Nitpick comments:
In @crates/cardano/src/ashard/loading.rs:

• Around line 19-57: The function load_pending_rewards_range currently accepts
  Option<Range> but the only caller (load_ashard) always passes
  Some(range), so tighten the signature to take Range directly: change
  the parameter type in load_pending_rewards_range and update callers (e.g., in
  load_ashard remove Some(range.clone()) and pass range.clone() directly); ensure
  the body drops any pattern-matching for None (no-op) and uses the range value
  when calling
  state.iter_entities_typed::(PendingRewardState::NS, range).

In @crates/cardano/src/ashard/rewards.rs:

• Around line 56-77: Update the stale comment referencing "EWRAP" to reflect the
  new pipeline stage: change the text around the account deregistration
  explanation to say "deregistered before AShard" (or "before the boundary's
  per-account leg") so it matches where the per-account registration check now
  runs; the relevant area to edit is the comment above the account.is_registered()
  branch in the function that calls reward.total_value() and
  ctx.rewards.return_reward_to_treasury(total). Ensure the updated comment keeps
  the existing clarification about pre-filtered accounts and returned_rewards
  behavior.

In @crates/cardano/src/ashard/shard.rs:

• Around line 37-60: The function shard_key_range uses debug_assert! to check
  validate_total_shards(total_shards) and shard_index < total_shards, which leaves
  a division-by-zero and silent truncation in release; change the debug_assert!
  checks to real runtime checks (e.g.,
  assert!(validate_total_shards(total_shards).is_ok()) and assert!(shard_index <
  total_shards)) or convert the function to return Result and propagate
  validate_total_shards() error from shard_key_range so PREFIX_SPACE /
  total_shards cannot divide by zero and invalid total_shards are handled
  deterministically; update callers if you choose the Result approach and keep
  references to shard_key_range, validate_total_shards, and PREFIX_SPACE when
  making the change.

In @crates/cardano/src/ashard/work_unit.rs:

• Around line 62-129: The load/commit_state duplication and drift risk can be
  fixed by capturing the computed total_shards and range during load and reusing
  them in commit_state instead of re-reading state: after computing total_shards
  and range in load (before calling BoundaryWork::load_ashard), store the values
  on the work unit (e.g., add fields self.total_shards and/or self.range and
  assign them there), and then change commit_state to take the boundary via
  self.boundary.as_mut() and use the stored range/total_shards when calling
  boundary.commit_ashard::(...) rather than recomputing via load_epoch; update
  BoundaryWork usage only to consume the already-stored range if needed.

In @crates/cardano/src/ewrap/commit.rs:

• Around line 91-100: The current call to stream_and_apply_namespace::<D,
  AccountState>(state, &writer, None) performs a full-table scan of AccountState
  each boundary; instead implement a targeted path that iterates over
  self.applied_mir_credentials (or the credentials from queued AssignRewards
  deltas), point-reads each AccountState, applies the MIR delta, and writes the
  result via the same writer so only affected accounts are touched; keep
  stream_and_apply_namespace for full-set namespaces like
  Pool/DRep/Proposal/EpochState, and ensure the new code reuses the existing
  apply/serialize logic used by stream_and_apply_namespace to maintain invariants
  and error handling.

In @crates/cardano/src/ewrap/loading.rs:

• Around line 280-285: BoundaryWork's shard_applied_effective /
  shard_applied_unspendable_to_treasury / shard_applied_unspendable_to_reserves
  are dead for the Ewrap path (zeroed in new_empty and never read by Ewrap; real
  per-shard accumulators live in EpochState.end via EpochEndAccumulate and are
  consumed by wrapup.flush), so fix by either splitting the dual-purpose struct
  into two clear types (e.g., AShardWork and EwrapWork) and move the AShard-only
  shard_applied_* fields into AShardWork, or add explicit documentation/comments
  on BoundaryWork and those three fields noting they are owned/used only by the
  AShard path; update new_empty, any constructors, and usages of BoundaryWork
  (including references in EpochEndAccumulate and wrapup.flush) to use the new
  types or to reflect the documented ownership to prevent future misuse.

In @crates/cardano/src/lib.rs:

• Around line 284-341: Extract and reuse the epoch load and the
  effective-account-shards logic: in initialize(), call load_epoch::(state)
  once and store the resulting EpochState (if Ok) for the warnings; replace the
  inline computation of effective_account_shards with a call to the existing
  helper read_effective_account_shards (or lift that helper to a free fn if
  necessary) so the "persisted total else config" rule is implemented in one
  place; update references to use the single loaded epoch value for the
  tracing::warn! blocks and derive effective_account_shards from
  read_effective_account_shards(state, &config) (or equivalent) to remove the
  duplicate load_epoch::(state) call.
• Around line 404-408: The code refreshes self.effective_account_shards on every
  pop_work by calling read_effective_account_shards::(domain.state()), but this
  value only matters when the epoch boundary handoff
  (EpochState.ashard_progress.total) can change; avoid the repeated read by first
  checking whether ashard_progress has changed and only calling
  read_effective_account_shards when it has. Add a small cached marker on the
  struct (e.g., last_ashard_total or last_ashard_progress) and in pop_work compare
  domain.state().ashard_progress.total (or the appropriate accessor) to that
  cached value; if different, call read_effective_account_shards and update both
  effective_account_shards and the cached marker, otherwise skip the read and keep
  the existing effective_account_shards.

In @crates/cardano/src/work.rs:

• Around line 276-279: Add a new unit test that sets TEST_TOTAL_SHARDS > 1
  (instead of the current const TEST_TOTAL_SHARDS = 1) to exercise the multi-shard
  sequencing in the pop_work logic: drive the WorkBuffer/state machine (invoking
  pop_work) and assert that it emits exactly N consecutive AShard units (N ==
  TEST_TOTAL_SHARDS) before producing an EwrapBoundary (or EWrap/Ewrap as used in
  this code), and verify that last_point_seen and any cursor invariants are
  preserved across the transition; target the test to exercise the pop_work path
  that contains the next_index >= total_shards branch and use the existing
  helpers/setup used by the other tests so it integrates with the same state
  machine and coverage.
• Around line 120-143: The code currently treats account_shards == 0 as a valid
  "no shards" path in on_ewrap_boundary (and similarly in the PreEwrapBoundary arm
  of pop_work), which contradicts validate_total_shards that requires
  account_shards >= 1; change the behavior to fail-fast: remove the special-case
  branch that returns EwrapBoundary when account_shards == 0 and instead assert or
  panic if account_shards < 1 (e.g., add assert!(account_shards >= 1) at the start
  of on_ewrap_boundary), keeping the normal Restart -> AShardingBoundary path for
  valid shard counts; alternatively, if the intended semantics are that 0 means
  "no shards", update validate_total_shards to accept 0 and document that
  invariant—pick one consistent approach and apply the same fix in the
  PreEwrapBoundary/pop_work code paths so both agree with validate_total_shards.

In @crates/cardano/work_units.md:

• Around line 5-11: The fenced code block containing the epoch diagram starting
  with "Estart → Roll … → Rupd → Roll … → AShard ×N → Ewrap" is missing
  a language tag (MD040); fix it by adding a language identifier (use "text")
  immediately after the opening triple backticks so the block becomes ```text and
  the diagram passes markdownlint-cli2 validation.

In @tests/epoch_pots/main.rs:

• Around line 500-513: Collapse the two separate if let Some(boundary) checks
  into a single binding for boundary when matching
  CardanoWorkUnit::AShard(shard): inside one if let Some(boundary) = shard.boundary() block first check if shard.shard_index() == 0 to clear
  accumulated_applied and set accumulated_ending_epoch = Some(boundary.ending_state().number), then unconditionally call
  accumulated_applied.extend(boundary.applied_rewards.iter().cloned()); this
  removes the duplicated boundary lookup and makes the reset-and-extend logic
  contiguous.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: defaults

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `13b48d06-fb3d-459b-b0a0-59ac2b808397`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 885094deed721abaa8bca14087703bd7ea409c28 and 8174d2a832bd81baa5228d8b3059b0b2e0bfd7ba.

</details>

<details>
<summary>📒 Files selected for processing (23)</summary>

* `crates/cardano/src/ashard/commit.rs`
* `crates/cardano/src/ashard/loading.rs`
* `crates/cardano/src/ashard/mod.rs`
* `crates/cardano/src/ashard/rewards.rs`
* `crates/cardano/src/ashard/shard.rs`
* `crates/cardano/src/ashard/work_unit.rs`
* `crates/cardano/src/ewrap/commit.rs`
* `crates/cardano/src/ewrap/loading.rs`
* `crates/cardano/src/ewrap/mod.rs`
* `crates/cardano/src/ewrap/work_unit.rs`
* `crates/cardano/src/ewrap/wrapup.rs`
* `crates/cardano/src/genesis/mod.rs`
* `crates/cardano/src/lib.rs`
* `crates/cardano/src/model/epochs.rs`
* `crates/cardano/src/model/mod.rs`
* `crates/cardano/src/model/pending.rs`
* `crates/cardano/src/rupd/work_unit.rs`
* `crates/cardano/src/work.rs`
* `crates/cardano/work_units.md`
* `crates/core/src/config.rs`
* `skills/debug-epoch-mismatch/SKILL.md`
* `tests/epoch_pots/main.rs`
* `tests/memory.rs`

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

crates/cardano/src/lib.rs (1)

250-258: Minor: deduplicate effective-shard-count computation.

The body of read_effective_account_shards (lines 253-258) is identical to the inline block at lines 345-348 in initialize. Since Self isn't yet constructed there, the method form can't be reused, but extracting a free helper (or Self::compute_effective_account_shards<D>(state, &config)) would keep both call sites in sync if the fallback rule ever changes.

♻️ Suggested refactor

-    /// Compute the effective `account_shards` value: stored
-    /// `ashard_progress.total` if a boundary is in flight, otherwise the
-    /// configured value.
-    fn read_effective_account_shards<D: Domain>(&self, state: &D::State) -> u32 {
-        load_epoch::<D>(state)
-            .ok()
-            .and_then(|e| e.ashard_progress.as_ref().map(|p| p.total))
-            .unwrap_or_else(|| self.config.account_shards())
-    }
+    /// Compute the effective `account_shards` value: stored
+    /// `ashard_progress.total` if a boundary is in flight, otherwise the
+    /// configured value.
+    fn compute_effective_account_shards<D: Domain>(state: &D::State, config: &CardanoConfig) -> u32 {
+        load_epoch::<D>(state)
+            .ok()
+            .and_then(|e| e.ashard_progress.as_ref().map(|p| p.total))
+            .unwrap_or_else(|| config.account_shards())
+    }
+
+    fn read_effective_account_shards<D: Domain>(&self, state: &D::State) -> u32 {
+        Self::compute_effective_account_shards::<D>(state, &self.config)
+    }

Then at line 345-348:

-        let effective_account_shards = load_epoch::<D>(state)
-            .ok()
-            .and_then(|e| e.ashard_progress.as_ref().map(|p| p.total))
-            .unwrap_or_else(|| config.account_shards());
+        let effective_account_shards =
+            Self::compute_effective_account_shards::<D>(state, &config);

Also applies to: 343-348

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/lib.rs` around lines 250 - 258, The effective-shard-count
computation is duplicated between the method read_effective_account_shards and
the inline block in initialize; extract the logic into a single helper (either a
free function like compute_effective_account_shards::<D>(state, &config) or an
associated fn Self::compute_effective_account_shards::<D>(state, &config)) that
returns the u32 by loading the epoch and falling back to
config.account_shards(), then replace both the body of
read_effective_account_shards and the inline compute in initialize to call that
helper so both call sites stay in sync if the fallback changes.

crates/cardano/src/ewrap/loading.rs (1)

159-165: Acknowledge the ordering constraint; consider tracking the TODO.

The comment clearly documents the rewards-before-drops ordering requirement and its rationale. The TODO to move retires to ESTART would eliminate this ordering hack.

Would you like me to open an issue to track this refactoring task (moving retires to ESTART after snapshot)?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/ewrap/loading.rs` around lines 159 - 165, Acknowledge the
rewards-before-drops ordering constraint and record the TODO by opening a
tracking issue: create an issue titled something like "Refactor: move retires to
ESTART to remove rewards-before-drops ordering hack" referencing the comment in
crates/cardano/src/ewrap/loading.rs, summarizing the current hack (rewards must
apply before drops because refunds clone live values pre-snapshot), and add
acceptance criteria that retires are moved to ESTART and the ordering
hack/comment removed; include links to the affected code and label it as a
refactor/tech-debt item for prioritization.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/core/src/work_unit.rs (1)

60-69: ⚠️ Potential issue | 🟠 Major

Reject 0 shards in the trait contract.

A u32 return still lets an implementation report 0 after initialize(), which means the executor can skip every load / compute / commit_* phase and jump straight to finalize(). Make this NonZeroU32 or add an explicit runtime check in the runner.

Based on learnings: Work units function as mini-ETL jobs that extract data from storage, transform it using chain-specific logic, and load results into appropriate stores following the sequence: load() → compute() → commit_wal() → commit_state() → commit_archive() → commit_indexes().

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/core/src/work_unit.rs` around lines 60 - 69, The trait currently
allows total_shards() to return 0 which skips all shard phases; fix by making
the contract non-zero: change the return type of total_shards() to
std::num::NonZeroU32 (update implementations to cache a NonZeroU32 in
initialize()), or if you prefer not to change the trait signature, add an
explicit runtime validation in the runner/executor (after calling initialize()
and reading total_shards()) that checks shards != 0 and returns an Err or panics
with a clear message; refer to the total_shards() method and initialize() call
sites in the executor/runner that loop over shards to add the check.

crates/cardano/src/estart/work_unit.rs (1)

93-106: ⚠️ Potential issue | 🔴 Critical

Resume from estart_progress.committed, and don't mask epoch read failures.

initialize() still restores only estart_progress.total. After a crash, the executor will rerun shards 0..k-1 unless it also resumes from committed, and this file already documents AccountTransition as non-idempotent. Falling back to ACCOUNT_SHARDS on load_epoch errors makes that restart path even less safe by silently repartitioning the boundary.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/estart/work_unit.rs` around lines 93 - 106, The
initialize() logic must resume from estart_progress.committed and must not
silently mask load_epoch failures; instead of unconditionally falling back to
ACCOUNT_SHARDS on Err(_), propagate the error and when load_epoch succeeds, set
self.total_shards from epoch.estart_progress.as_ref().map(|p|
p.total).unwrap_or(ACCOUNT_SHARDS) and also set the worker's resume point from
epoch.estart_progress.as_ref().and_then(|p| p.committed) (e.g., assign to
whatever field tracks the next shard to run), so shards restart from the
committed position; remove the Err(_) => ACCOUNT_SHARDS branch and return the
DomainError on read failure.

crates/cardano/src/shard.rs (1)

76-79: ⚠️ Potential issue | 🟠 Major

Validate shard counts in release builds too.

These guards disappear in release, but total_shards now comes from persisted ShardProgress.total as well as the constant. A bad value can still panic on PREFIX_SPACE / total_shards or produce broken partitions at runtime, so this needs a real assert! or Result path instead of debug_assert!.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/shard.rs` around lines 76 - 79, The debug-only checks in
shard_key_ranges are unsafe; replace the debug_assert! calls with real runtime
validation: call validate_total_shards(total_shards) and return/propagate an
error on failure (or use assert! if you prefer panicking) and explicitly check
shard_index < total_shards, returning Err for invalid inputs; ensure callers
that read persisted ShardProgress.total handle the Result. Update
shard_key_ranges (and its callers) to use the new Result-returning signature or
use assert! so that division by PREFIX_SPACE / total_shards and partitioning
logic cannot run with invalid total_shards.

crates/cardano/src/model/epochs.rs (1)

780-783: The expect() on entity.end remains fragile.

Per the past review comment, this expect() relies on Genesis seeding end = Some(EndStats::default()) and EpochTransition re-seeding it on every subsequent transition. While the comment was marked as addressed, the code still uses expect() rather than the suggested get_or_insert_with(EndStats::default) approach, which would be more defensive.

This is low-risk since the invariant is maintained by the current code paths, but consider the defensive approach for robustness against future regressions.

🛡️ Optional defensive change

-        let end = entity
-            .end
-            .as_mut()
-            .expect("ESTART seeded EpochState.end before shards run");
+        let end = entity.end.get_or_insert_with(EndStats::default);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/model/epochs.rs` around lines 780 - 783, Replace the
fragile expect() on entity.end with a defensive get_or_insert_with to ensure
EndStats is initialized if missing; specifically, change the code path in the
epochs handling where entity.end is accessed (currently using
.end.as_mut().expect("ESTART seeded EpochState.end before shards run")) to call
.end.get_or_insert_with(EndStats::default) or equivalent so the EndStats default
is guaranteed without panicking, leaving the rest of the logic in the
EpochTransition/epochs handling unchanged.

crates/cardano/src/lib.rs (1)

349-448: Consider adding crash-recovery check for rupd_progress.

The initialization checks ewrap_progress and estart_progress for crash-recovery scenarios, but rupd_progress is not checked. Since RUPD also uses the same sharded pattern with RupdProgress deltas, operators would benefit from similar logging if a crash occurred mid-RUPD.

📝 Suggested addition after line 447

// Same crash-recovery check for the RUPD phase.
if let Some(progress) = epoch.rupd_progress.as_ref() {
    let configured = crate::shard::ACCOUNT_SHARDS;
    if progress.total != configured {
        tracing::warn!(
            epoch = epoch.number,
            stored_total = progress.total,
            configured_total = configured,
            "in-flight RUPD uses {} shards but ACCOUNT_SHARDS = {}; \
             the in-flight RUPD will continue with {} (the persisted total)",
            progress.total,
            configured,
            progress.total,
        );
    }
    if progress.committed < progress.total {
        tracing::warn!(
            epoch = epoch.number,
            next_shard = progress.committed,
            total_shards = progress.total,
            "crash detected mid-RUPD: rupd_progress is set. \
             On the next RUPD trigger, dolos will resume the pipeline."
        );
    }
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/lib.rs` around lines 349 - 448, Add the same
crash-recovery logging for rupd_progress that exists for ewrap_progress and
estart_progress: after loading epoch (load_epoch::<D> and the epoch variable),
check epoch.rupd_progress.as_ref(), compare progress.total against
crate::shard::ACCOUNT_SHARDS and emit a tracing::warn with epoch, stored_total,
configured_total when they differ, and if progress.committed < progress.total
emit a tracing::warn including next_shard and total_shards warning that a crash
was detected mid-RUPD; mirror the style and fields used in the existing
ewrap/estart blocks (use progress, progress.total, progress.committed, and
epoch.number).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/cardano/src/model/epochs.rs (2)

1490-1497: Remove unused #[allow(deprecated)] attribute.

Static analysis correctly identifies this attribute as unused. Outer attributes on prop_compose! macro invocations don't propagate into the macro expansion. The test function at line 1660 that calls any_epoch_wrap_up() already has its own #[allow(deprecated)], which is where the suppression actually takes effect.

🧹 Suggested fix

-    #[allow(deprecated)]
     prop_compose! {
         fn any_epoch_wrap_up()(
             stats in any_end_stats(),
         ) -> EpochWrapUp {
             EpochWrapUp::new(stats)
         }
     }

As per coding guidelines: "Run cargo clippy --workspace --all-targets --all-features and resolve all clippy warnings before committing changes".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/model/epochs.rs` around lines 1490 - 1497, Remove the
unused outer attribute by deleting the #[allow(deprecated)] placed immediately
above the prop_compose! invocation that defines any_epoch_wrap_up; the
suppression does not propagate into the macro expansion and the test calling
any_epoch_wrap_up already has its own #[allow(deprecated)], so keep that test
attribute and remove the one before prop_compose! (symbols to locate:
prop_compose!, any_epoch_wrap_up, any_end_stats, EpochWrapUp::new).

---

1603-1613: Remove unused #[allow(deprecated)] attribute.

Same issue as line 1490 — this outer attribute on the macro invocation is ineffective. The test function at line 1685 already has the necessary #[allow(deprecated)].

🧹 Suggested fix

-    #[allow(deprecated)]
     prop_compose! {
         fn any_epoch_transition()(
             new_epoch in root::any_epoch(),
         ) -> EpochTransition {
             // new_pots is filled in by the test harness from the entity's initial_pots
             // so that `new_pots.max_supply() == entity.initial_pots.max_supply()` holds
             // (which `apply`'s debug_assert requires).
             EpochTransition::new(new_epoch, crate::pots::Pots::default(), None, None)
         }
     }

As per coding guidelines: "Run cargo clippy --workspace --all-targets --all-features and resolve all clippy warnings before committing changes".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/cardano/src/model/epochs.rs` around lines 1603 - 1613, Remove the
ineffective outer attribute #[allow(deprecated)] applied to the prop_compose!
macro invocation that defines any_epoch_transition; simply delete that attribute
so the macro block starts with prop_compose! { ... } and leave the rest
(new_epoch in root::any_epoch(), EpochTransition::new(...)) unchanged — the test
function already carries the needed #[allow(deprecated)] where necessary.